Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Serialization fixes #64

Closed
wants to merge 2 commits into from
Closed

Serialization fixes #64

wants to merge 2 commits into from

Conversation

bowald
Copy link
Contributor

@bowald bowald commented Apr 9, 2018

Summury

  • Skips identity matrices for nodes during serialization.
  • Skips empty ExtensionMap for materials during serialization
  • Null values in ExtensionMap are serialized as empty object {} instead of being skipped (An extension witch contain null as value is KHR_materials_unlit )
  • SerializeGltfParameter outputs Factor if numberArrays size is 1.
  • Clang format tiny_gltf.h

To easier see my changes, see commit 6e96b12, first commit is only clang format

Discussion

Should default values (such as identity matrices) be skipped during serialization? Or is it up to the user of tinygltf to input correct data?

@syoyo
Copy link
Owner

syoyo commented Apr 10, 2018

Should default values (such as identity matrices) be skipped during serialization?

Yes, we can safely skip serializing default values where JSON schema defines it : https://github.com/KhronosGroup/glTF/blob/master/specification/2.0/schema/node.schema.json

@bowald
Copy link
Contributor Author

bowald commented Apr 10, 2018

Yes, we can safely skip serializing default values where JSON schema defines it

Good, I will keep adding checks against default values during serialization.

@syoyo
Copy link
Owner

syoyo commented May 29, 2018

@bowald Any updates?

@bowald
Copy link
Contributor Author

bowald commented Jun 4, 2018

@syoyo I'm sorry, been busy. will have some time to make fixes in the near coming weeks.

@bowald bowald closed this Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bowald @syoyo